🔴 Critical vis_types/types_service.ts:53

In the deprecated createBaseVisualization method, async () => new BaseVisType(config) constructs a BaseVisType instance that is then passed to registerVisualization, which constructs another BaseVisType around it—creating a double-wrapped object. The registerVisualization function expects a factory returning Promise<VisTypeDefinition>, but receives a factory returning Promise<BaseVisType>. This causes the inner BaseVisType to be treated as the raw config object, duplicating defaults and breaking type expectations.

       createBaseVisualization: <TVisParams extends VisParams>(
         config: VisTypeDefinition<TVisParams>
       ): void => {
-        this.registerVisualization(config.name, async () => new BaseVisType(config));
+        this.registerVisualization(config.name, async () => config);
       },

In file src/platform/plugins/shared/visualizations/public/vis_types/types_service.ts around lines 53-57:

In the deprecated `createBaseVisualization` method, `async () => new BaseVisType(config)` constructs a `BaseVisType` instance that is then passed to `registerVisualization`, which constructs another `BaseVisType` around it—creating a double-wrapped object. The `registerVisualization` function expects a factory returning `Promise<VisTypeDefinition>`, but receives a factory returning `Promise<BaseVisType>`. This causes the inner `BaseVisType` to be treated as the raw config object, duplicating defaults and breaking type expectations.

Evidence trail:
- PR diff for types_service.ts: git_diff base=MERGE_BASE head=REVIEWED_COMMIT path=src/platform/plugins/shared/visualizations/public/vis_types/types_service.ts
- types_service.ts lines 27-37 (registerVisualization): creates `new BaseVisType(config)` from the factory result
- types_service.ts line 53 (createBaseVisualization): passes `async () => new BaseVisType(config)` as the factory → double wrapping
- types_service.ts lines 60-66 (createBaseVisualizationAsync): correctly passes factory directly, no double wrapping
- base_vis_type.ts constructor: applies `defaultsDeep` for visConfig, editorConfig, options — these would be called twice on the same data in the double-wrap case

@nreese this comment seems legit, in the registerVisualization function you are calling:

this.types[name] = async () => {
      const config = await getVisDefinition();
      return new BaseVisType(config);
    };

so basically new BaseVisType(new BaseVisType(config))

@nreese this comment seems legit, in the registerVisualization function you are calling:

this.types[name] = async () => {
      const config = await getVisDefinition();
      return new BaseVisType(config);
    };

so basically new BaseVisType(new BaseVisType(config))

If you change this, in the Dashboard Add from library you will lose the chart icons.
Not a big deal for the agg-base visualize charts, but Vega will be affected to.

Before:

After:

If you change this, in the Dashboard Add from library you will lose the chart icons.
Not a big deal for the agg-base visualize charts, but Vega will be affected to.

I think this is ok and even beneficial in the long run. There are plans to create a vega embeddable in the near future for "as code". This change will make it clear when a vega library item is vega embeddable or legacy vega vis.

I see but for now I don't think this is a valid change, it will create confusion until we don't have the vega embeddable is ready. We should not release this change until Vega is not clearly distinguishable from the rest.

You can just hardcode a check for the visState.type and return the Vega icon if vega and visualizeApp otherwise. We don't really need the registry for that. We can then clean this up with the vega embeddable

nit: The asyncMap propagates the first rejection, so if a dynamic import fails everything is rejected (this differs a bit from the previous logic if I understood it correcly)

How would you like this resolved? Promise.all?